Add an sensor for the tonal palette for dynamic colors - #7319
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the existing Dynamic Color (accent color) sensor to expose the system “theme style” (tonal palette variant) alongside the already-reported seed/accent color, enabling Home Assistant themes to generate palettes consistent with the phone’s dynamic color style.
Changes:
- Read
android.theme.customization.theme_stylefromSettings.Securetheme overlay JSON and publish it as a newvariantattribute on the accent color sensor. - Add Robolectric/Hilt tests covering sensor availability, state updates, RGB attribute output, and the new
variantattribute behavior (including malformed input).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| app/src/main/kotlin/io/homeassistant/companion/android/sensors/DynamicColorSensorManager.kt | Adds reading/parsing of theme style from system settings and publishes it as a new sensor attribute. |
| app/src/test/kotlin/io/homeassistant/companion/android/sensors/DynamicColorSensorManagerTest.kt | Adds unit tests validating dynamic color sensor behavior and the new variant attribute. |
Suppressed comments (1)
app/src/main/kotlin/io/homeassistant/companion/android/sensors/DynamicColorSensorManager.kt:103
- PR description mentions adding an additional sensor, but the code change here adds a new attribute ("variant") to the existing dynamic color sensor update. If no separate sensor was intended, consider updating the PR summary/description to match the implemented behavior to avoid confusion for reviewers and docs.
mapOf(
"rgb_color" to listOf(accent.red, accent.green, accent.blue),
"variant" to variantThemeStyle.orEmpty(),
),
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Thanks for the suggestion. The use of attributes is strongly discouraged in Home Assistant nowadays. As this doesn't describe the color, it could be a full sensor instead, I think. We can then also provide a proper description, and send all possible values to the server, to improve the user experience. Manually reading JSON from undocumented system settings feels like it could be error-prone, especially as multiple manufacturers implement Dynamic color differently. Do you have any additional sources or reports that indicate this is stable and works on a wide variety of devices? What about simply exposing all theme colors? (Doing a quick search in Android source code I can't find a documented API or attribute for the palette enum either.) |
Ha, I actually originally had it as an additional sensor and then thought the attribute approach was the way to go. Happy to change that back, but want to get your input first on the rest of your comment. I would prefer to use a declared constant, but
Tests are here. I wouldn't recommend treating it as an enum with a fixed set of values, even if the CTS tested values are all currently enumerated. It is possible that future versions of Android (or other OEMs) would expand on this. Anyone reading from the sensor would be expected to handle unknown values (at least the way I envision it). Thanks for the review! Let me know your thoughts and if you think it's a practical way forward I'll change to an additional sensor. Edit: As far as surfacing all colors, that is possible but would be quite expansive (currently 65 colors). |
Thanks for linking to the CTS test, that does give some confidence in the setting and its results. I didn't check it in the first place but indeed, it does use the same format (JSON).
Sending all known options doesn't lock the sensor to behave like an enum and reject unknown values. However it does provide users with a nice dropdown of all known options for their automations, and generates warnings on the server if a different value is sent which allows us to discover and document them. Without it requires users to review the documentation to know what to expect at all. I'm almost certain we'll discover more as I already have a choice of 9 palettes on my Pixel - or we discover that this value isn't detailed enough.
OK I agree 65 sensors isn't ideal 😅 |
5874420 to
2aeda6f
Compare
Got it, thanks for the clarification! I've pushed the updated changes as well as new screenshots in the PR description. I updated the sensor logic to encode the list of options (let me know if I did this correctly -- I didn't see these appear as options when creating an automation, but was following what I saw for other enum-type sensors). I added a bit more safeguarding when reading / parsing the JSON data just to be sure there is specific warning messaging for various situations.
Regarding this, these are actually all limited to |
This adds an additional sensor to pair with the current Dynamic Color sensor for the seed color. In conjunction with the seed color, a theme style is used to determine the generated color palette. This combination allows for shared color definitions between Home Assistant and the phone's theme. In the case of an error with parsing the JSON data, the state will be rendered as UNKNOWN.
2aeda6f to
e7a6d70
Compare
b92469f to
c0b8e13
Compare
Summary
This adds an additional sensor to the current Dynamic Color sensor for the tonal palette variant. In conjunction with the seed color, this is used to determine the generated color palette.
This combination allows for shared color definitions between Home Assistant and the phone's theme. The motivation here was that I was creating a theme that would use the Material Color Utilities library to create a color palette that would align with the phone's theme. This allows the app to blend in with native applications more effectively.
The seed color (already exposed as a sensor) is not entirely sufficient, since the generated palette can vary widely based on the selected theme style.
Checklist
Select exactly one option that describes AI usage in this contribution:
Screenshots
Link to pull request in documentation repositories
User Documentation: home-assistant/companion.home-assistant#1403
Any other notes